Skip to content

Conversation

@mucahitkls
Copy link
Contributor

Pull Request Description:

This pull request introduces two key efficiency enhancements to the Modbus plugin, designed to reduce unnecessary data traffic and allow for more flexible polling strategies.These changes enable users to prioritize high-frequency data from critical devices while minimizing network and cloud resource usage for static or infrequently changing data points.

Motivation

In many industrial environments, it's necessary to monitor certain devices (like critical sensors) at a high frequency (e.g., every second), while other devices only require infrequent updates. Similarly, some registers, such as status flags, only need to be reported when their value changes. This implementation addresses that need by providing more granular control over data polling and publishing.

Features Implemented

1. Per-Device Polling Interval

This feature allows a custom polling interval to be set for each individual Modbus device, overriding the global default.

  • How it Works: The plugin checks for an optional poll_interval key within a device's configuration in the devices.toml file. If this key is present, its value is used for that specific device. If it is absent, the plugin falls back to the global pollinterval defined in modbus.toml, ensuring full backward compatibility.
  • Configuration: To set a custom rate, add poll_interval = <seconds> to a [[device]] table in devices.toml.

2. On-Change Publishing

This feature prevents the plugin from publishing data for a register if its value has not changed since the last poll.

  • How it Works: For any register configured with on_change = true, the plugin caches the last published value. A new MQTT message is only published if the newly polled value is different from the cached one, or on the very first poll for that register. If the on_change key is false or absent, the default behavior of publishing after every poll is maintained.
  • Configuration: To enable this, add on_change = true to a [[device.registers]] table in devices.toml.

Example Configurations

Test modbus.toml:

[modbus]
ip_address = "127.0.0.1"
port = 502
pollinterval = 5
loglevel = "INFO"

[thinedge]
mqtthost= "127.0.0.1"
mqttport = 1883

Test devices.toml with new updates:

[[device]]
name = "mock_battery"
address = 1
ip = "127.0.0.1"
port = 502
protocol = "TCP"
poll_interval = 1  # new update
[[device.registers]]
number = 0
startbit = 0
nobits = 16
signed = false
multiplier = 1
input = false
measurementmapping.templatestring = '{"power": %%}'

[[device.registers]]
number = 100
startbit = 0
nobits = 16
signed = false
input = false
measurementmapping.templatestring = '{"error_flag": %%}'
on_change = true # new update

@mucahitkls mucahitkls temporarily deployed to Test Pull Request July 14, 2025 17:44 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented Jul 14, 2025

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
15 0 0 15 100 31.629714s

Passed Tests

Name ⏱️ Duration Suite
Device should support the operation c8y_ModbusConfiguration 0.102 s Device
Set values via c8y_ModbusConfiguration Operation 2.319 s Operation
Poll rate and transmit rate should be updated for the Device 0.192 s Operation
Poll rate and transmit rate should be updated on the Device 2.315 s Operation
Device should support the operation c8y_ModbusDevice 0.102 s Device
Device should have installed software tedge-modbus-plugin 0.126 s Debian
Service should be active 2.319 s Debian
ReInstall Modbus Plugin 22.793 s Debian
Device should have the fragment c8y_ModbusConfiguration 0.201 s Device
ChildDevice TestCase1 should be created 0.190 s Device
ChildDevice TestCase1 should have the fragment c8y_ModbusDevice 0.481 s Device
Service tedge-modbus-plugin should be enabled 0.100 s Device
ChildDevice TestCase1 should have a Test.Int16 Measurement 0.196 s Telemetry
ChildDevice TestCase1 should have a Test.Float32 Measurement 0.194 s Telemetry
ChildDevice TestCase1 should have Alarms of type TestAlarm on Coil Value 1 0.001 s Telemetry

@reubenmiller
Copy link
Contributor

@mucahitkls Thanks for the PR and the very thorough description explaining the new behaviour. Any chance that you could also add a tests, either unit tests (python) or system units (Robot Framework). If you need assistance, then we can help out.

@reubenmiller
Copy link
Contributor

@mucahitkls Can you also run the formatting and linting using the following commands: (though I'm pretty sure the formatter (black) after looking at the lint errors)

just format
just lint

@reubenmiller reubenmiller added the enhancement New feature or request label Jul 14, 2025
The  class was using a class-level variable for its  cache, which is intended to store the last-published values for the on-change publishing feature.

This created a bug where a single cache dictionary was shared across all  instances. When polling multiple devices, the mapper for one device would read or overwrite the cached values of another. This interference caused the on-change logic to behave unpredictably, particularly failing to publish a value on the first poll because it incorrectly found a stale value from a different device's polling cycle.

This issue was discovered when unit tests for the on-change feature consistently failed, as the shared state persisted between test cases.

The fix moves the  cache initialization into the  method, converting it from a shared class variable to a private instance variable. This ensures that each mapper instance—and therefore each Modbus device being polled—has its own isolated cache.

This change corrects the on-change publishing behavior to be reliable and robust on a per-device basis and allows unit tests to pass successfully.
Adds unit test suites for the ModbusMapper and ModbusPoll classes to validate the new on-change and per-device polling features.

These tests verify two key areas:

-   **On-Change Publishing ():**
    This suite confirms that the  correctly publishes MQTT messages only when a register's value has changed. It covers scenarios for integers, floats, initial polls, and the fallback behavior.

-   **Per-Device Polling ():**
    This suite uses mocking to ensure the  scheduler uses the device-specific  when available and correctly falls back to the global  when it is not.

These tests provide confidence in the correctness of the new features and protect against future regressions.
@mucahitkls mucahitkls temporarily deployed to Test Pull Request July 15, 2025 11:34 — with GitHub Actions Inactive
@mucahitkls
Copy link
Contributor Author

Hello,

Unit tests have been added to the project to validate the new efficiency features: per-device polling intervals and on-change publishing.

During the development of these tests, a possible bug was discovered in the ModbusMapper class. The cache used for the on-change feature was implemented as a shared class variable instead of an instance variable. This affected the general plugin by causing a single cache to be shared across all devices being polled. As a result, if multiple devices were configured, the on-change logic for one device could be incorrectly triggered or suppressed by the values from another, leading to unreliable behavior.

It is resolved by converting the cache to an instance variable within the mapper's __init__ method. This fix ensures each device operates with its own isolated cache, making the on-change feature reliable and improving the overall robustness of the plugin.

@reubenmiller
Copy link
Contributor

@mucahitkls Looks like the macOS preview file (.DS_Store) made it into the PR before you added the gitignore rule.

@mucahitkls mucahitkls temporarily deployed to Test Pull Request July 15, 2025 13:56 — with GitHub Actions Inactive
@mucahitkls
Copy link
Contributor Author

hello, sorry for inconvenience, now it should be fine

@reubenmiller reubenmiller changed the title Feat/efficiency enhancements: Implement per-device polling and on-change publishing feat: Implement per-device polling and on-change publishing Jul 15, 2025
Copy link
Contributor

@reubenmiller reubenmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mucahitkls Looks good and thanks for taking the time to also add unit tests

@reubenmiller
Copy link
Contributor

@mucahitkls Sorry just spotted a few minor things (more of a consistency thing). https://github.com/thin-edge/modbus-plugin/pull/28/files#r2207606778

…perties are added in devices.toml with commented out
@mucahitkls mucahitkls temporarily deployed to Test Pull Request July 15, 2025 14:32 — with GitHub Actions Inactive
@mucahitkls
Copy link
Contributor Author

please let me know if it need to be updated

@reubenmiller reubenmiller added this pull request to the merge queue Jul 15, 2025
Merged via the queue into thin-edge:main with commit 923de41 Jul 15, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants